Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Вставлять mock для BEM.I18N до контента шаблона #92

Closed
wants to merge 1 commit into from

Conversation

rndD
Copy link
Contributor

@rndD rndD commented Jun 23, 2015

Привет.
В BH с версии 4.x.x больше нет прямой связи с BEM.I18N. Теперь если есть желание работать с переводами в BH нужно сделать: bh.lib.i18n = BEM.I18N.

При текущей реализации mock-lang-js.js сделать это не получается, так как BEM.I18N объявлен позже. Я перенес его в место перед кодом шаблонов.

@levonet levonet added the review label Jun 23, 2015
@blond
Copy link
Member

blond commented Jun 23, 2015

Привет!

С проблемой согласен. Перед тем как влить — попытаюсь понять, что может сломать такое изменение.

@scf2k, не помнишь почему в этом PR поменял mock и content местами?

@scf2k
Copy link
Contributor

scf2k commented Jun 23, 2015

Там написано :)

  •            // if there is a #sourceMappingURL pragma append
    
  •            // the mock before it so the source map will be
    
  •            // valid. We can't insert it in the beginning because
    
  •            // source map locations will point to the wrong lines.
    

@blond
Copy link
Member

blond commented Jun 23, 2015

@scf2k, оу, спасибо! :)

Нам же sourcemap'ы нужны для подсчёта покрытия шаблонов тестами? Мы не можем BEM.I18N просто обернуть в игнорируемые коментарии /* istanbul ignore next */?

@scf2k
Copy link
Contributor

scf2k commented Jun 25, 2015

Я не знаю что мы можем, но на момент приклеивания i18n мап уже сгенерен. Соответственно если мы что-то добавляем в начало или где-то внутри, то мап ломается. Не понял почему ты спрашиваешь про истанбул - вроде бы он тут никак не фигурирует.

@blond
Copy link
Member

blond commented Jul 1, 2015

@rndD, изменения в таком виде принять не могу, т.к. они сломают уже работающее поведение.

Завёл задачу — #94, будем думать как исправить.

Этот PR закрываю, если появится другое решение или идеи — пиши в задачу или присылай новый PR. В любом случае спасибо за попытку! :)

@blond blond closed this Jul 1, 2015
@blond blond removed the review label Jul 1, 2015
@rndD
Copy link
Contributor Author

rndD commented Jul 1, 2015

@blond спасибо, если пойму в чем проблема, попробую решить.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants